Skip to content

node: remove potential deadlock on chan capacity of zero#1116

Merged
MSevey merged 1 commit into
mainfrom
sevey/if-cleanup
Jul 31, 2023
Merged

node: remove potential deadlock on chan capacity of zero#1116
MSevey merged 1 commit into
mainfrom
sevey/if-cleanup

Conversation

@MSevey

@MSevey MSevey commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Overview

There was a potential deadlock for a channel capacity of zero. This code removes that deadlock and adds a comment explaining the code more.

Related to #1069

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@MSevey MSevey added the T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. label Jul 31, 2023
@MSevey MSevey self-assigned this Jul 31, 2023
@MSevey MSevey enabled auto-merge July 31, 2023 14:06
@codecov

codecov Bot commented Jul 31, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 64.40% and project coverage change: +0.47% 🎉

Comparison is base (9460a18) 55.52% compared to head (f880268) 55.99%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1116      +/-   ##
==========================================
+ Coverage   55.52%   55.99%   +0.47%     
==========================================
  Files          63       61       -2     
  Lines        6808     6552     -256     
==========================================
- Hits         3780     3669     -111     
+ Misses       2622     2491     -131     
+ Partials      406      392      -14     
Files Changed Coverage Δ
config/config.go 89.65% <ø> (-0.67%) ⬇️
node/full_client.go 44.26% <0.00%> (-0.19%) ⬇️
rpc/json/test_helpers.go 100.00% <ø> (ø)
block/manager.go 9.43% <8.33%> (+0.54%) ⬆️
node/full.go 64.57% <100.00%> (-0.01%) ⬇️
node/light.go 60.00% <100.00%> (+9.54%) ⬆️
node/test_helpers.go 78.94% <100.00%> (-1.70%) ⬇️
state/executor.go 55.52% <100.00%> (+5.40%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tuxcanfly tuxcanfly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@MSevey MSevey added this pull request to the merge queue Jul 31, 2023
Merged via the queue into main with commit 4c903bc Jul 31, 2023
@MSevey MSevey deleted the sevey/if-cleanup branch July 31, 2023 15:04
Manav-Aggarwal pushed a commit that referenced this pull request Sep 6, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview
There was a potential deadlock for a channel capacity of zero. This code
removes that deadlock and adds a comment explaining the code more.

Related to #1069 

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants